Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write current state to a statefile (preparation for the future systray applet) #148

Merged
merged 6 commits into from
May 7, 2024
Merged

Conversation

trigg
Copy link
Contributor

@trigg trigg commented May 7, 2024

When changing state write it to a statefile.

By default install to and use /var/lib/arch-update/state, but allow User or System to specify with env variable ARCH_UPDATE_STATE set to a file path.

This is in preparation of a system tray icon, and is the method of telling the icon to change.

Later on we could expand it to have more information than a simple icon-name, but it will suffice as a first step.

Addresses parts of #146

@trigg
Copy link
Contributor Author

trigg commented May 7, 2024

I've opted to not include a remove line for icons just yet as it might confuse automatic merging.

@Antiz96 Antiz96 self-requested a review May 7, 2024 14:21
@Antiz96 Antiz96 added this to the v2.0.0 milestone May 7, 2024
@Antiz96
Copy link
Owner

Antiz96 commented May 7, 2024

Are we able to use the already existing statedir mechanism implemented in users' $XDG_STATE_HOME dir instead of using /var/lib/arch-update/state?

That will only dropping the change in the Makefile and the addition of the "res/state" file as well as avoiding having another file write-able by world (666 mode).

@trigg
Copy link
Contributor Author

trigg commented May 7, 2024

Good point. I have no idea how I missed that.

src/script/arch-update.sh Outdated Show resolved Hide resolved
src/script/arch-update.sh Outdated Show resolved Hide resolved
res/state Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
src/script/arch-update.sh Outdated Show resolved Hide resolved
src/script/arch-update.sh Outdated Show resolved Hide resolved
src/script/arch-update.sh Outdated Show resolved Hide resolved
@Antiz96
Copy link
Owner

Antiz96 commented May 7, 2024

I've opted to not include a remove line for icons just yet as it might confuse automatic merging.

If you are referring to dropping "superfluous" icon (like "checking" and "installing"), I'll include that in the incoming "dropping icon changes" PR ;)

@Antiz96
Copy link
Owner

Antiz96 commented May 7, 2024

Good point. I have no idea how I missed that.

Hey hey, no problem don't worry 😄
Thanks for adapting your changes!

@trigg
Copy link
Contributor Author

trigg commented May 7, 2024

If you are referring to dropping "superfluous" icon

No, sorry. We still have an icon directory variable knocking around. Since it's a line edited by one of the other PRs I'll hold of on editing it in here too

@Antiz96
Copy link
Owner

Antiz96 commented May 7, 2024

If you are referring to dropping "superfluous" icon

No, sorry. We still have an icon directory variable knocking around. Since it's a line edited by one of the other PRs I'll hold of on editing it in here too

Yeah okay, fair enough. I'll drop it myself in another PR a bit later today and I'll rebase other PRs if needed.

- update comments
src/script/arch-update.sh Outdated Show resolved Hide resolved
src/script/arch-update.sh Outdated Show resolved Hide resolved
src/script/arch-update.sh Outdated Show resolved Hide resolved
Copy link
Owner

@Antiz96 Antiz96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Antiz96
Copy link
Owner

Antiz96 commented May 7, 2024

Ready to be merged.

Here again I'm waiting for my incoming PR about dropping the icon changing to sync the merge of the different PR (to avoid having breaking changes in the main branch for too long).

Thanks once again for your awesome work!

@trigg
Copy link
Contributor Author

trigg commented May 7, 2024

Anytime! I look forward to your critique of the next PR! 😀

@Antiz96
Copy link
Owner

Antiz96 commented May 7, 2024

Anytime! I look forward to your critique of the next PR! 😀

I'm currently making a first read of it to acknowledge the content and I'll eventually leave a few comments if I have some, but I'll probably make an actual review later this evening as I'll be AFK for a few hours right now 😛

I'll also add you as a reviewer of the "dropping changing icon" PR since it's all part of the same effort, if that's okay for you!

@trigg
Copy link
Contributor Author

trigg commented May 7, 2024

I'll also add you as a reviewer of the "dropping changing icon" PR

Sounds good to me!

I'll be AFK for a few hours right now

No worries! I'll be AFK myself for large swathes for the next few days, should be able to reply on github but will be slower on commits.

@Antiz96 Antiz96 changed the title Write state to a statefile Write current state to a statefile (preparation for the future systray applet) May 7, 2024
@Antiz96 Antiz96 merged commit 0de8419 into Antiz96:main May 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants